Conversation
Add AndroidPhone/AndroidTablet/AndroidAmbiguous variants emitting the
single-byte ASCII letters 'e', 'd', 'f' that the official WhatsApp
Android client uses for companion_platform_id during pair-code/QR
companion registration.
The previous fallback (OtherWebClient = '9' + display "Chrome (Android)")
passed server validation but was idiomatic for web clients only. The
server's accept-list for companion_platform_id is 23 single-byte ids
('0'..'9' for web, 'a'..'m' for native); the three Android letters are
the only mobile mappings observed empirically so far.
Behaviour changes when the caller has set
device_props.platform_type to one of the Android variants:
AndroidPhone '9' + "Chrome (Android)" -> 'e' + "Android (<os>)"
AndroidTablet '9' + "Chrome (Android)" -> 'd' + "Android (<os>)"
AndroidAmbiguous '9' + "Chrome (Android)" -> 'f' + "Android (<os>)"
iOS, Wear OS, AR/VR and Cloud API platforms still fall back to
OtherWebClient ('9') because no client has been reverse-engineered yet
to confirm which letter (a, b, c, g..k) maps to each of them.
Drops the (incorrect) "server validates display strictly as
Browser (OS) with Browser in {Chrome, Edge, Firefox, IE, Opera, Safari}"
claim from docs and tests. Empirically the server only validates the
display field length (1..=100) and accepts arbitrary content.
Breaking changes (intentional, pre-1.0):
- CompanionWebClientType::code() -> i32 replaced by wire_byte() -> u8
- CompanionWebClientType is no longer #[repr(i32)] with discriminants
📝 WalkthroughWalkthroughThis pull request refactors the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@wacore/src/companion_reg.rs`:
- Around line 19-57: Replace the manual match-based wire mapping with a
WireEnum-derived enum: add #[derive(WireEnum, Debug, Clone, Copy, Default,
PartialEq, Eq, Hash)] to CompanionWebClientType and annotate every variant with
its wire value (e.g. #[wire = b'0'] on Unknown, #[wire = b'1'] on Chrome, etc.),
remove the explicit match body inside wire_byte() and make wire_byte() a thin
helper that returns the variant's wire value via the WireEnum implementation (or
remove it if redundant), and ensure you do not derive
serde::Serialize/Deserialize or add serde renames so the #[wire = ...]
attributes are the single source of truth for the protocol mapping.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e5abbc1d-fc19-4159-8fb7-b7cd261937a1
📒 Files selected for processing (3)
wacore/src/companion_reg.rswacore/src/pair.rswacore/src/pair_code.rs
Comment on lines
19
to
57
| #[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Hash)] | ||
| #[repr(i32)] | ||
| pub enum CompanionWebClientType { | ||
| // Web (digit codes from WAWebCompanionRegClientUtils.DEVICE_PLATFORM). | ||
| #[default] | ||
| Unknown = 0, | ||
| Chrome = 1, | ||
| Edge = 2, | ||
| Firefox = 3, | ||
| Ie = 4, | ||
| Opera = 5, | ||
| Safari = 6, | ||
| Electron = 7, | ||
| Uwp = 8, | ||
| OtherWebClient = 9, | ||
| Unknown, | ||
| Chrome, | ||
| Edge, | ||
| Firefox, | ||
| Ie, | ||
| Opera, | ||
| Safari, | ||
| Electron, | ||
| Uwp, | ||
| OtherWebClient, | ||
| // Mobile (letter codes from the official WhatsApp Android client). | ||
| AndroidTablet, | ||
| AndroidPhone, | ||
| AndroidAmbiguous, | ||
| } | ||
|
|
||
| impl CompanionWebClientType { | ||
| pub const fn code(self) -> i32 { | ||
| self as i32 | ||
| /// Single-byte ASCII id placed in `<companion_platform_id>`. | ||
| pub const fn wire_byte(self) -> u8 { | ||
| match self { | ||
| Self::Unknown => b'0', | ||
| Self::Chrome => b'1', | ||
| Self::Edge => b'2', | ||
| Self::Firefox => b'3', | ||
| Self::Ie => b'4', | ||
| Self::Opera => b'5', | ||
| Self::Safari => b'6', | ||
| Self::Electron => b'7', | ||
| Self::Uwp => b'8', | ||
| Self::OtherWebClient => b'9', | ||
| Self::AndroidTablet => b'd', | ||
| Self::AndroidPhone => b'e', | ||
| Self::AndroidAmbiguous => b'f', | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Put the wire value on the enum itself.
This is protocol surface area, so the byte mapping needs to live on the variants, not in a separate manual match. Right now CompanionWebClientType and wire_byte() can drift independently, which is exactly the failure mode the repo rule is trying to prevent. Please move this enum to #[derive(WireEnum)] with per-variant #[wire = ...] annotations, and keep wire_byte() only as a thin helper if you still want that API.
As per coding guidelines, "Every protocol enum must use #[derive(WireEnum)]; the #[wire = "..."] or #[wire = NUM] attribute is the single source of truth for each variant's wire value; do not also derive serde::Serialize/Deserialize or add #[serde(rename_all)]".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@wacore/src/companion_reg.rs` around lines 19 - 57, Replace the manual
match-based wire mapping with a WireEnum-derived enum: add #[derive(WireEnum,
Debug, Clone, Copy, Default, PartialEq, Eq, Hash)] to CompanionWebClientType and
annotate every variant with its wire value (e.g. #[wire = b'0'] on Unknown,
#[wire = b'1'] on Chrome, etc.), remove the explicit match body inside
wire_byte() and make wire_byte() a thin helper that returns the variant's wire
value via the WireEnum implementation (or remove it if redundant), and ensure
you do not derive serde::Serialize/Deserialize or add serde renames so the
#[wire = ...] attributes are the single source of truth for the protocol
mapping.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
AndroidPhone/AndroidTablet/AndroidAmbiguousvariants toCompanionWebClientType, emitting the single-byte ASCII letters'e','d','f'(matching the official WhatsApp Android client).wa::DeviceProps::PlatformType::AndroidPhone/Tablet/Ambiguousto those variants incompanion_web_client_type_for_platform(instead of falling back toOtherWebClient)."Android (<os>)"ascompanion_platform_displayfor those variants (instead of the"Chrome (<os>)"fallback).<Browser> (<OS>)with browser ∈ {Chrome, Edge, Firefox, IE, Opera, Safari}" claim from docs and tests. The server only validates length 1..=100 and accepts arbitrary content.Why
The server accepts 23 single-byte ids in
companion_platform_id: digits0..9(WAWeb'sCompanionWebClientType) and lettersa..m(native clients). Of the 13 letters, three map to confirmed Android variants based on official-client RE:defPreviously, callers setting
platform_type = AndroidPhonegot'9'(OtherWebClient) +"Chrome (Android)". Both passed server validation by luck, but neither matches what the real Android client emits. This PR aligns with the empirical wire format.The remaining 10 letters (
a,b,c,g..m) are accepted by the server but no client has been RE'd to confirm which platform each represents. Adding speculative variants risks mislabelling the device on the primary side, so they are intentionally omitted. Future PRs can add iOS / Mac / Wear OS / AR / Quest / smartglasses letters as binary RE or live captures confirm them.Behaviour changes
When a caller sets
device_props.platform_typeto an Android variant:For all other variants (web browsers, Desktop, UWP, iOS, AR/VR, etc.), behaviour is unchanged.
Breaking changes (intentional, pre-1.0)
CompanionWebClientType::code() -> i32is replaced bywire_byte() -> u8. The new method returns the raw ASCII byte (b'1',b'e', ...) rather than an integer encoding.Displaynow formats the byte as a char.CompanionWebClientTypeis no longer#[repr(i32)]with explicit discriminants. The wire mapping is centralised inwire_byte().